Software factory change - #546
Merged
Merged
Conversation
`flows logs <run> --step <agent-step>` rendered Claude's `stream-json` and
printed one `frame … (not rendered here — see --raw)` line per frame for a
Codex step. The software-factory preset runs its reviewers on Codex, so the
verdict that matters most in a run was unreadable without `--raw` and hand
parsing.
`cloud-transcript.ts` now dispatches per frame rather than per provider, and
`cloud-transcript-codex.ts` reads the `codex exec --json` vocabulary:
`thread.started`, `turn.started`/`turn.completed`/`turn.failed`, a top-level
`error`, and `item.started`/`item.updated`/`item.completed` for
`agent_message`, `reasoning`, `command_execution`, `file_change`,
`mcp_tool_call` and `error`.
- Calls are numbered within their attempt and carry the result size, the
exit code (zero included), the item's status and a bounded output excerpt
(10 lines / 1,000 characters), so a failed command keeps its size, its
exit and its failure on one line rather than collapsing to `ERROR`.
- Agent messages print in full: the last one is the step's answer.
- `reasoning` is a character count and never its text, as Claude's
`thinking` already is.
- Item lifecycles are matched locally and in order -- within an attempt,
thread and turn, by id *and* item type -- so a reused id never reaches
across a boundary and attempt 1's unfinished command survives attempt 2
completing the same id. An unfinished call is shown at its last snapshot
and marked as never completed.
- Nothing is dropped: `web_search`, `todo_list`, an unknown item type and a
malformed known item all keep the placeholder line, now naming the item
type, and a malformed item never spends a call number.
- Every provider string is redacted whole before it is bounded, and every
newly rendered string goes through the control-character filter.
Claude entries are unchanged, field for field and line for line; everything
Codex adds is a new union member or an optional field.
The fixtures are captured, not written: `codex-exec-json.jsonl` is one real
`codex exec --json` run of codex-cli 0.155.1 with its thread id and working
directory normalised, and `codex-exec-json-failures.jsonl` is frames selected
from four more captured runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`mcpEntry` serialized an MCP call's arguments and redacted the serialization. `redact` matches an environment value literally, and `JSON.stringify` escapes a quote, backslash, newline or tab inside one -- so a secret carrying any of them no longer matched its own value once serialized, and reached the tool line and the `--json` entry escaped but complete. The credential-field-by-name rule does not cover it: the value sits in an ordinary field such as `text` or `content`. Rendering MCP arguments is what newly exposed this; the frame was a placeholder before. Arguments are now redacted as decoded leaves -- values and keys, at every depth, through arrays and a bare string argument -- and the serialized form is still redacted again by the `bounded` call, which is what the credential-field rule needs, since a leaf standing alone has no field name left to recognise. The line is bounded only after both passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Relayflow: the adversarial review did not pass. This branch is not approved: the flow stopped here and did not mark it ready to merge. Review of PR #546Reviewed head: No new actionable code findings in the eight-file diff against Verdict: verification blocked; no new code findings. ScopeRead AGENTS.md and RFC-0001; reviewed the full PR diff, both fixtures, shared PR discussionCommands (run from packages/sdk): gh pr view 546 --json body,headRefOid,comments,reviews > /tmp/codex-review-current/pr.json
gh api --paginate repos/AgentWorkforce/flows/pulls/546/comments > /tmp/codex-review-current/inline-comments.jsonRead the PR description and all returned comments/reviews. The issue comment Captured discussion fields (body omitted here; full response at the path above): {
"headRefOid": "349eeb69e34632cbf34b847571a44d82bf67ae2d",
"comments": [
{
"id": "IC_kwDOUF0ysM8AAAABV9L0MQ",
"author": {
"login": "coderabbitai"
},
"authorAssociation": "NONE",
"body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> ## Review skipped\n> \n> Bot user detected.\n> \n> To trigger a single review, invoke the `@coderabbitai review` command.\n> \n> <details>\n> <summary>\u2699\ufe0f Run configuration</summary>\n> \n> **Configuration used**: Organization UI\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Advanced\n> \n> **Run ID**: `ed8332bd-158e-4bd0-a732-5fbf45b80ec9`\n> \n> </details>\n> \n> You can disable this status message by setting the `reviews.review_status` to `false` in the CodeRabbit configuration file.\n> \n> Use the checkbox below for a quick retry:\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> \ud83d\udd0d Trigger review\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=AgentWorkforce/flows&utm_content=546)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>\u2764\ufe0f Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)\n- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)\n- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)\n- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)\n\n</details>\n\n\n<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>\n\n<!-- tips_end -->",
"createdAt": "2026-09-21T22:29:57Z",
"includesCreatedEdit": true,
"isMinimized": false,
"minimizedReason": "",
"reactionGroups": [],
"url": "https://github.com/AgentWorkforce/flows/pull/546#issuecomment-5768410161",
"viewerDidAuthor": false
}
],
"reviews": []
}Inline comments: []Focused verificationCommand, from packages/sdk: npx vitest run tests/cloud-transcript-codex.test.ts tests/cloud-read.test.ts > /tmp/codex-review-current/focused.log 2>&1Captured output: Renderer mutation proofDriver: Command: Captured output (including exact test commands): Prior P1 regression mutation proofDriver: Command: Captured output: Full affected-package verification — unresolvedCommand, from packages/sdk: npm test > /tmp/codex-review-current/sdk-test.log 2>&1Exit status: 1. The run reported 8 failed files, 41 failed tests, and one This is a verification limitation, not a finding that the transcript diff Complete captured output: npm test output |
khaliqgant
marked this pull request as ready for review
September 22, 2026 18:44
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 349eeb69e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+216
to
+218
| seq, status: status === null ? null : bounded(status, context.clean, 40), | ||
| exit_code: null, output_excerpt: null, output_truncated: false, complete, | ||
| error: failure === null ? null : bounded(failure, context.clean, ERROR_MAX_CHARS), |
There was a problem hiding this comment.
When an mcp_tool_call returns textual result.content, this branch calculates only its character count and always sets output_excerpt to null, so the rendered transcript shows something like → 14 chars but hides the actual tool result. This contradicts the new rendering contract that MCP calls include a bounded output excerpt and forces readers back to --raw for potentially important MCP evidence; extract and redact/bound the result text as is done for command output.
Useful? React with 👍 / 👎.
`mcpEntry` measured an mcp_tool_call's result and threw the text away, so a reader got a size and had to re-fetch with --raw. It now bounds and redacts the result exactly as `commandEntry` bounds command output: text content, the MCP content-block array, or a structured-only result serialized. Addresses the review finding on cloud-transcript-codex.ts:218. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
|
Fixed in 3f19fda.
Five new tests cover text, string, block-skipping, truncation and redaction, and the two existing MCP assertions were updated for the excerpt line. Mutation-proved — reverting With the fix restored: 39 passed (39). |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3f19fda. Configure here.
| if (parts.length > 0) return parts.join('\n'); | ||
| // A result can be structured only; showing it beats reporting a size alone. | ||
| const structured = result['structured_content']; | ||
| return structured === undefined || structured === null ? null : JSON.stringify(structured); |
There was a problem hiding this comment.
Structured results skip leaf redaction
Medium Severity
mcpResultText runs JSON.stringify on structured_content and only then hands the bytes to excerpt. redact matches an environment value literally, so a secret containing a quote, backslash, newline, or tab no longer matches once escaped and reaches both the page and --json. MCP arguments already run redactLeaves first for this reason; content-block text is unaffected because it is still a raw string.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3f19fda. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
flows logsrenders Codex transcriptsflows logs <run> --step <agent-step>rendered Claude'sstream-jsonand, fora Codex step, printed one
frame … (not rendered here — see --raw)line perframe. The software-factory preset runs its reviewer steps on Codex, so the
verdict that matters most in a run could not be read without
--rawplus handparsing.
It now renders. This is a read-only SDK projection: no kernel change, no change
to what the worker writes, no change to the
trajectory_taildigest or toflows status --cloud.What changed
packages/sdk/src/cloud-transcript-types.tspackages/sdk/src/cloud-transcript-codex.tscodex exec --jsonvocabulary — lifecycle matching, entry construction, and the render cases the Codex shapes own.packages/sdk/src/cloud-transcript.tspackages/sdk/tests/cloud-transcript-codex.test.tspackages/sdk/tests/cloud-read.test.tspackages/sdk/tests/fixtures/codex-exec-json*.jsonldocs/CLOUD.mdSizes against AGENTS §1: 330, 379 and 214 lines. Nothing approaches 500.
The rendering contract
Dispatch is per frame, not per provider, so a log that mixes vocabularies
renders each in its own shape and a Codex log with no
relayflow.attemptwrapper is still recognised as a transcript.
thread.started→session codex · thread <id>.turn.started/turn.completed→ separators; the completion carriesinput/output, cached input, cache write and reasoning-output tokens. A
missing value stays absent or
?— no fabricated zero usage, no cost.agent_message→ prose in full, redacted, newlines preserved. The finalone is the step's answer. No digest cap, no verdict heuristic.
reasoning→ a character count only, exactly as Claude'sthinkingis.Its text never reaches the page.
command_executionandmcp_tool_call→ numbered call lines carryingthe result size, the exit code (zero included), the item's status, an
incompleteness marker, and a bounded output excerpt: 10 lines or 1,000
characters after redaction, with an explicit cut marker. The original size
stays on the call line, separate from the excerpt. A failed call keeps size,
exit and failure text together — it does not collapse to Claude's
ERROR.file_change→ every path and change kind. File activity, not a call, so ittakes no number. Malformed change elements are counted, never dropped.
turn.failed, a top-levelerrorand anerroritem → the redactedmessage under a failure label. Nothing that reads like success.
(
item.completed/todo_list).web_searchandtodo_listare deliberatelyleft there: naming an item type is not knowing which of its fields carry what.
Lifecycle matching
Ordered and local, not a global set of completed ids. Attempt markers (both
kinds),
thread.startedand every turn boundary partition matching; within apartition an item is keyed by id and type, and a start after a completion
of that key opens a new lifecycle. So:
no completion frame;Call numbers restart at each attempt and run on across turns and threads.
Public API
TranscriptEntrygainsthread,turn,errorandfile_changemembers;TranscriptToolgains an optionalcodexsub-object andTranscriptMessagean optional
complete. Additive: a Claude entry serializes to the bytes itserialized to before — pinned by a test. A consumer switching exhaustively over
kindmust add the new members.Provenance of the fixtures
Captured on this machine on 2026-09-21 with
codex-cli 0.155.1(
codex --version→codex-cli 0.155.1), logged in via ChatGPT.packages/sdk/tests/fixtures/codex-exec-json.jsonl— one run, verbatim exceptthat its
thread_idand its/tmp/codexrevworking directory were replaced bystable placeholders:
captured output (
cut -c1-400 /tmp/codexrev/capture.jsonl):normalisation, the whole of it:
packages/sdk/tests/fixtures/codex-exec-json-failures.jsonl— frames selectedfrom four further captured runs in the same session: a
reasoningitem; acommand that exited 1 (
cat no-such-file.txt); an MCP call the demo serverrefused; and
codex exec --json -m no-such-model-xyz, which is what producedthe
erroritem, the top-levelerrorframe andturn.failed. Their threadids were normalised and item ids renumbered so the concatenation reads as one
thread. That last capture, verbatim:
The event and item names are also the ones the codex-cli binary's own enums
carry —
stringsover@openai/codex-linux-x64/vendor/.../bin/codexliststhread.started,turn.started,turn.completed,turn.failed,item.started,item.updated,item.completed,error, the item tagsagent_message reasoning command_execution file_change mcp_tool_call web_search todo_list,the statuses
in_progress|completed|failed, and the usage keys.item.updatedwas not observed in any capture. It is in that event enum, soits handling is pinned by a synthetic frame and the test says so in as many
words. Everything else asserted here came out of a real run.
The render, before and after
Before, for a Codex step — the shape the issue reported, every frame a
placeholder:
After (
packages/sdk/tests/fixtures/codex-exec-json.jsonlthroughrenderAgentTranscript):Verification
Targeted
Typecheck
Full package suite — not green here, and not because of this change
npm test(sh scripts/test.sh:test:prep,typecheck,build,typecheck:tests,vitest run) exits 1 in this environment:Every one of those 39 failures is a missing kernel binary.
ops/cargo.shbuilds into
$RELAYFLOWS_TOOLCHAIN_HOME/target/<key>/debug, and these testsspawn the hardcoded in-repo path:
The distinct spawn errors in that run's output, deduplicated:
I measured the baseline rather than asserting it. With this branch's changes
stashed (
git stash -u), the same seven files on the same machine:Same seven files, same 39 failures, same per-file counts
(
live-kernel8,provider-trigger-executor3,stuck-run-triage22,webhook-live6, plusauthored-node-runtime,canonical-software-factoryand
mcp). This change adds no failure. It does not make the suite green,and I am not claiming it does: the full package suite did not pass here, and
CI is the place that verdict should come from.
Mutation proof
The exact implementation bytes were saved outside the worktree first
(
cp src/cloud-transcript*.ts /tmp/impl-save/), so each restore is a bytecomparison against the passing implementation rather than a checkout to HEAD.
Tests were not touched between the failure and the pass. Every cycle below is
apply → targeted run fails → restore →
cmp→ identical command passes.M1 — remove the Codex dispatch branch, so every Codex frame falls to the placeholder
including, literally:
restored and rerun:
M2 — drop the call number, the exit code and the output excerpt from the render
restored (
identical:for all three files) and each identical command rerun:M3 — suppress Codex file-change rendering (
case 'file_change': return [])restored (
identical:× 3), rerun:M4 — cap the assistant message at 80 characters on one line
The suffix the issue names is exactly what the mutation loses. Restored
(
identical:× 3), rerun:M5 — bypass redaction on the command-output excerpt
The secret is synthetic (
secret-+ 1,200x+-tail, exported asDEPLOY_TOKENin the test's explicit env).Note what survives the mutation: the command target, the path and the
turn.failedmessage are all still[redacted:DEPLOY_TOKEN]. Only the excerptleaks, which is exactly the branch the mutation removed. Restored
(
identical:× 3), rerun:After all five
The tree that was committed is the tree these five cycles were run against.
Not done, deliberately
trajectory_taildigest.agent-transcript.tsstill counts Codexitem types and reports
tools.complete: false; its comment says "until afixture pins the frame shapes", and those shapes are now pinned. Pairing
calls properly there is a separate change, a separate gate, a separate PR.
web_searchandtodo_list. They keep the placeholder. Their item tagsare known; which of their fields carry what is not, from a capture I have.
need a kernel binary at a path
ops/cargo.shdoes not produce here. The sameseven fail identically on the branch point. See above.
Note
Medium Risk
Read-only CLI/SDK projection with broad new parsing and redaction paths for agent logs; mistakes could leak secrets or misrepresent failures, but Claude output is regression-tested and behavior is heavily fixture-covered.
Overview
flows logs --stepnow renders Codexcodex exec --jsontranscripts instead of one “not rendered here” placeholder per frame, so software-factory reviewer verdicts are readable without--raw.The SDK splits shared transcript entry types into
cloud-transcript-types.tsand addscloud-transcript-codex.tsfor parsing and terminal rendering: threads and turns (with usage), full agent messages, reasoning as a char count only, numbered shell/MCP calls with exit codes and bounded redacted output excerpts, file changes, and explicit error lines.cloud-transcript.tskeeps Claude behavior and gains per-frame dispatch to Codex; mixed logs render each vocabulary in its own shape. Item lifecycles mergeitem.started/updated/completedwithin attempt/thread/turn partitions.The public
TranscriptEntryunion grows additively (thread,turn,error,file_change, optionalcodexon tools). Docs add a Codex example; 34 unit tests, 2 CLI tests, and captured fixtures pin real codex-cli shapes and redaction (including MCP args redacted before JSON stringify).Reviewed by Cursor Bugbot for commit 3f19fda. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Makes
flows logs <run> --step <agent-step>render Codex transcripts instead of one placeholder per frame, so software-factory reviewer verdicts no longer need--raw. The change is a read-only SDK projection: it does not touch the kernel, what the worker writes, thetrajectory_taildigest, orflows status --cloud.What changed
cloud-transcript-codex.ts) and shared entry types (cloud-transcript-types.ts); dispatch is per frame, so Claude entries render exactly as before and mixed logs keep each shape.reasoningstays a character count.--json.thread,turn,error, andfile_changemembers additively.codex-cli 0.155.1, plus a worked example indocs/CLOUD.md.Explicitly deferred
trajectory_taildigest still treats Codex items as tools incomplete; updating it is a separate change.web_searchandtodo_listkeep the placeholder until their field shapes are pinned.Written for commit 3f19fda. Summary will update on new commits.